Skip to content

Added Migurski/ OSM Terrain for the US#406

Closed
rsignell-usgs wants to merge 4 commits intoSciTools:masterfrom
rsignell-usgs:master
Closed

Added Migurski/ OSM Terrain for the US#406
rsignell-usgs wants to merge 4 commits intoSciTools:masterfrom
rsignell-usgs:master

Conversation

@rsignell-usgs
Copy link
Contributor

Since I work for the USGS, I thought it would be cool to add the US Terrain tile map service that uses the USGS National Elevation Database to create this cool terrain tile map service.

Example:
http://nbviewer.ipython.org/github/rsignell-usgs/notebook/blob/master/Cartopy/Cartopy_terrain.ipynb

I'm not sure about the best name for this new class. I called it "StamenTerrain" instead of "Terrain" because stamen is providing this terrain service and there could be others. But I yield to your judgement here.

BTW, Maybe since there are a lot of these services that follow the same pattern
http://wiki.openstreetmap.org/wiki/List_of_OSM_based_Services
would it be useful to have a generic class for these services that a user could instantiate with a base url?

@pelson
Copy link
Member

pelson commented Apr 29, 2014

👍 - it'd be useful to put a docstring on the class, just to clarify that this is only defined over the US (sadly, as the rendering is beautiful).

I certainly think we can streamline the construction of some of these services - I'm not too worried about doing that in this PR, but please feel free to investigate as a follow on.

There are a few PEP8 test failiures from this PR:

test_pep8_conformance (cartopy.tests.test_pep8.TestCodeFormat) ... /usr/local/lib/python2.7/dist-packages/cartopy/io/img_tiles.py:226:1: W293 blank line contains whitespace
/usr/local/lib/python2.7/dist-packages/cartopy/io/img_tiles.py:227:1: E302 expected 2 blank lines, found 1
/usr/local/lib/python2.7/dist-packages/cartopy/io/img_tiles.py:232:80: E501 line too long (82 > 79 characters)
/usr/local/lib/python2.7/dist-packages/cartopy/io/img_tiles.py:235:1: E302 expected 2 blank lines, found 1

@rsignell-usgs
Copy link
Contributor Author

Okay, updated for PEP8 compliance and added a docstring describing this US coverage. Note that the tools for creating these tiles are at https://github.com/migurski/DEM-Tools and at least the Canadians have used them: migurski/DEM-Tools#4
The lower zoom levels are from SRTM worldwide coverages (coverages: NED10m, NED100m, NED1km, SRTM1, SRTM3)

@rsignell-usgs rsignell-usgs changed the title Added Mizurski/ OSM Terrain for the US Added Migurski/ OSM Terrain for the US Apr 29, 2014
@rsignell-usgs
Copy link
Contributor Author

Should we call this OSMTerrain or USTerrain instead of StamenTerrain?

@rsignell-usgs
Copy link
Contributor Author

Are there python editors that test pep8 compliance? It seems like using Travis CI to find pep8 errors is a bit of a waste...

@rsignell-usgs
Copy link
Contributor Author

@pelson, So I see that this PR still fails, but is this something I need to fix?

======================================================================
FAIL: cartopy.tests.mpl.test_images.test_web_tiles
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/local/lib/python2.7/dist-packages/nose/util.py", line 618, in newfunc
    return func(*arg, **kw)
  File "/usr/local/lib/python2.7/dist-packages/cartopy/tests/mpl/__init__.py", line 210, in wrapped
    self.run_figure_comparisons(figures, test_name=mod_name)
  File "/usr/local/lib/python2.7/dist-packages/cartopy/tests/mpl/__init__.py", line 150, in run_figure_comparisons
    self.do_compare(result_path, expected_path, self.tolerance)
  File "/usr/local/lib/python2.7/dist-packages/cartopy/tests/mpl/__init__.py", line 184, in do_compare
    assert False, msg
AssertionError: Images were different (RMS: 1.80599137902).
/home/travis/build/SciTools/test_folder/cartopy_test_output/test_images/result-web_tiles.png /usr/local/lib/python2.7/dist-packages/cartopy/tests/mpl/baseline_images/mpl/test_images/web_tiles.png /home/travis/build/SciTools/test_folder/cartopy_test_output/test_images/result-web_tiles-failed-diff.png
Consider running idiff to inspect these differences.

@ajdawson
Copy link
Member

The local tests you run before submitting your PR should pick up the PEP8 errors so you can get them cleared up before you push your branch.

You can also run pep8 yourself (pip install pep8 or similar) and then pep8 filename.py which will check the whole file (fine if it was already PEP8 compliant and you made changes) or

git diff -U0 upstream/master | pep8 --diff

to just check the lines you changed relative to the upstream master branch.

You can configure many editors to do PEP8 checking, I generally don't bother and instead rely on the locally run test suite and/or manual checking, but that is personal preference.

@ajdawson
Copy link
Member

So I see that this PR still fails, but is this something I need to fix?

I think the Google tiles have changed (ocean names have moved position) since the static image was created. I don't believe this is due to your code so you are safe to ignore it.

@rsignell-usgs
Copy link
Contributor Author

Thanks for the tutorage, @ajdawson !

@rsignell-usgs
Copy link
Contributor Author

so okay to merge?

@rhattersley
Copy link
Member

Is the "build.out" file supposed to be included?

@rhattersley
Copy link
Member

Thanks @rsignell-usgs - I've rebased and squashed these onto master as 727d0ed. (I've assumed "build.out" was just a mistake so it's not included.)

@rhattersley rhattersley closed this May 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants